Refactor CARLA Operator #279
Conversation
| if isinstance(other, Pose): | ||
| return (self.transform == other.transform | ||
| and self.forward_speed == other.forward_speed | ||
| and self.velocity_vector == other.velocity_vector) |
There was a problem hiding this comment.
| if isinstance(other, Pose): | |
| return (self.transform == other.transform | |
| and self.forward_speed == other.forward_speed | |
| and self.velocity_vector == other.velocity_vector) | |
| if isinstance(other, Pose): | |
| return (self.transform == other.transform | |
| and self.forward_speed == other.forward_speed | |
| and self.velocity_vector == other.velocity_vector) | |
| return NotImplemented |
| def matches_data(left_msgs, right_msgs): | ||
| length_matches = (len(left_msgs) == len(right_msgs)) | ||
| return length_matches and all( | ||
| map( | ||
| lambda x: x[0].timestamp == x[1].timestamp and x[0].data == x[1]. | ||
| data, zip(left_msgs, right_msgs))) |
There was a problem hiding this comment.
Is this for shadow testing and will be removed once the PR is approved?
There was a problem hiding this comment.
Yeah, this is to check that the messages sent and received are the same.
| class CarlaObstaclesDriverOperator(CarlaBaseGNSSDriverOperator): | ||
| """Publishes the bounding boxes of all vehicles and people retrieved from | ||
| the simulator at the provided frequency. |
There was a problem hiding this comment.
Why were these operators subclassed from a BaseGNSSDriverOperator? They don't seem to be using the gnss_msg exposed in the process_gnss method and retrieve different data from the simulator.
There was a problem hiding this comment.
My thought process is the following:
- The original CARLA operator performs complicated logic to send data from the simulator (e.g. pose, ground obstacles, etc) at multiple frequencies (localization and control). This logic is coupled with how the operator ticks the simulator.
- Sensors can define tick frequencies independently. Therefore, we can set up a lightweight sensor such as GNSS which calls the logic for sending arbitrary information at arbitrary frequencies.
- The logic for setting up the GNSS sensor is identical across several operators, so I moved the shared logic to
BaseGNSSDriverOperator. I could also move it to a helper method or renameprocess_gnss, if you think that's cleaner.
I checked if any other actors can invoke callbacks at configurable frequencies, but it appears that this is limited to sensors.
There was a problem hiding this comment.
So the GNSS sensor is being used as a timestamping mechanism? Why not just have a base operator that sends its downstream operators a watermark message to retrieve and release data?
There was a problem hiding this comment.
That works too, but it would still share logic with the GNSS sensor operator, and we'd still need to instantiate one for each downstream operator frequency. So the question is whether the data release mechanism should be a standalone operator or a coupled with the driver operators.
Personally, I think it makes sense to put the logic in the driver operator so that the API reflects the other driver operators and simplifies pipeline architecture.
| gnss_setup = pylot.drivers.sensor_setup.GNSSSetup( | ||
| self.config.name, transform) | ||
| super().__init__(vehicle_id_stream, obstacles_stream, gnss_setup, | ||
| frequency, flags) |
There was a problem hiding this comment.
Does this mean that a new GNSS will be installed for every operator?
Refactors the CARLA operator by creating drivers for pose, ground truth information, and HD maps. The
CarlaOperatorwill continue to write onvehicle_id_streamonce the ego-vehicle is set up.Changes
Driver Operators
Tested via the
MatchesOperatoron synchronous and pseudo-async mode.CarlaPoseDriverOperator. Implementspose_streamandpose_stream_for_control.CarlaTrafficLightsDriverOperator. Implementsground_traffic_lights_stream.CarlaObstaclesDriverOperator. Implementsground_obstacles_stream.CarlaSpeedLimitSignsDriverOperator. Implementsground_speed_limit_signs_stream.CarlaStopSignsDriverOperator. Implementsground_stop_signs_stream.CarlaOpenDriveDriverOperator. Implementsopen_drive_stream.Other Changes
__eq__forPosefor testing.CarlaOperatorglobal_trajectory_streamfrom theCarlaOperator. Replace with anIngestStreamas it only sends a top watermark.MatchesOperatoronce all drivers are tested.